Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Tests/EntitlementsSpec.swift (1)

292-325: Convert this spec to AsyncSpec before using await.

These examples call await expect { try await … }, but the spec still subclasses QuickSpec, so the compiler rejects the async calls (same problem noted previously). Switch to AsyncSpec and make spec() async—or drop the await expectations.

-class EntitlementsSpec: QuickSpec {
-    override class func spec() {
+class EntitlementsSpec: AsyncSpec {
+    override class func spec() async {

Also update any beforeEach/it bodies to await as needed.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Sources/KindeSDK/APIs/PermissionsAPI.swift (1)

10-46: Same refactoring opportunities as RolesAPI.

PermissionsAPI has the same structure and SwiftLint warnings as RolesAPI. The recommendations from RolesAPI.swift apply here as well:

• Consider using a caseless enum instead of final public class
• Extract common request builder logic to reduce duplication

Based on static analysis hints from SwiftLint.

Sources/KindeSDK/APIs/FeatureFlagsAPI.swift (1)

10-46: Same refactoring opportunities as RolesAPI and PermissionsAPI.

FeatureFlagsAPI follows the same pattern as the other API clients. Apply the same refactoring recommendations mentioned for RolesAPI.swift.

Based on static analysis hints from SwiftLint.

Tests/EntitlementsSpec.swift (1)

319-355: Async expectations require AsyncSpec or waitUntil.

The await expect { try await ... } pattern inside a synchronous QuickSpec will not compile. Swift requires the enclosing context to be async to use await.

Convert to AsyncSpec or use waitUntil blocks:

-class EntitlementsSpec: QuickSpec {
+class EntitlementsSpec: AsyncSpec {

Or wrap each async test in waitUntil:

it("throws notAuthenticated when user is not logged in") {
    waitUntil { done in
        Task {
            do {
                _ = try await entitlementsService.fetchEntitlements()
                fail("Expected error to be thrown")
            } catch {
                expect(error).to(matchError(AuthError.notAuthenticated))
            }
            done()
        }
    }
}

Sources/KindeSDK/Models/RolesResponse.swift (1)

3-28: LGTM - Well-structured response model.

The response model properly handles optional data and provides helpful utility methods. The use of compactMap correctly filters out roles with null keys.

Note: The isValid() implementation is duplicated across RolesResponse, PermissionsResponse, and FeatureFlagsResponse. Consider extracting this to a protocol for consistency and maintainability.

Sources/KindeSDK/APIs/RolesAPI.swift (2)

10-24: Address SwiftLint warning: prefer static methods.

The final public class with only class methods pattern triggers SwiftLint warnings. Since RolesAPI has no instance state, consider one of these approaches:

1. Use a caseless enum (prevents instantiation):

-final public class RolesAPI {
+public enum RolesAPI {
+    
+    @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *)
+    public static func getRoles() async throws -> RolesResponse {
-    final public class func getRoles() async throws -> RolesResponse {

2. Or use static methods directly on the class and add a private initializer to prevent instantiation.

Note: Apply the same fix to PermissionsAPI and FeatureFlagsAPI for consistency.

Based on static analysis hints from SwiftLint.

---

26-46: Implementation is correct but highly duplicated.

The request builder pattern is properly implemented with authentication required. However, this code is nearly identical to PermissionsAPI and FeatureFlagsAPI, differing only in the endpoint path and return type. Consider extracting a generic request builder helper to reduce duplication.

Sources/KindeSDK/Models/PermissionsResponse.swift (1)

1-52: LGTM - Consistent with RolesResponse pattern.

PermissionsResponse follows the same well-structured pattern as RolesResponse. As noted in the RolesResponse review, the isValid() method is duplicated across all response models (Permissions, Roles, FeatureFlags). Consider extracting this to a protocol for better maintainability.

Tests/EntitlementsSpec.swift (2)

349-355: Inconsistent indentation in nested describe blocks.

The getEntitlementsDictionary and Hard Check Methods describe blocks have extra indentation compared to sibling blocks. This appears to be a formatting oversight.

-                describe("getEntitlementsDictionary") {
-                    it("throws notAuthenticated when user is not logged in") {
+            describe("getEntitlementsDictionary") {
+                it("throws notAuthenticated when user is not logged in") {

---

41-42: TODO comments should be tracked for follow-up.

Static analysis flagged unresolved TODO comments. Consider creating issues to track adding proper test fixtures with mocked tokens, or mark these tests as pending with a clear reason.

Would you like me to help create GitHub issues to track these test fixture improvements?

Also applies to: 263-264

Sources/KindeSDK/Models/FeatureFlagsResponse.swift (1)

37-47: Consider logging unknown flag types for debugging.

Unknown flag types are silently skipped, which is safe but may make debugging difficult if the API introduces new types.

             let flagType: Flag.ValueType?
             switch item.type {
             case "Boolean":
                 flagType = .bool
             case "String":
                 flagType = .string
             case "Integer":
                 flagType = .int
             default:
+                // Silently skip unknown types - consider logging in debug builds
                 continue
             }

Sources/KindeSDK/Auth/Auth.swift (5)

1046-1073: Duplicate ValueType enum definitions in Flag and FeatureFlag.

Both Flag and FeatureFlag structs define identical ValueType enums with the same raw values and behavior. This duplication increases maintenance burden and risks divergence.

Consider extracting a shared type:

/// Shared enum for feature flag value types
public enum FlagValueType: String, Codable {
    case string = "s"
    case int = "i"
    case bool = "b"
    
    public var typeDescription: String {
        switch self {
        case .string: return "string"
        case .bool: return "boolean"
        case .int: return "integer"
        }
    }
}

Then update both structs to use FlagValueType instead of their nested enums.

Also applies to: 1089-1124

---

1407-1415: Redundant nil initialization.

SwiftLint correctly flags that var startingAfter: String? = nil is redundant since optionals default to nil.

     public func getAllEntitlements() async throws -> [Entitlement] {
         var allEntitlements: [Entitlement] = []
-        var startingAfter: String? = nil
+        var startingAfter: String?
         
         repeat {
             let response = try await fetchEntitlements(startingAfter: startingAfter)

---

1337-1343: HTTP requests lack timeout configuration.

The URLRequest objects don't set timeoutInterval, relying on system defaults (60 seconds). For production SDK usage, explicit timeouts provide better control over failure modes.

         var request = URLRequest(url: url)
         request.httpMethod = "GET"
+        request.timeoutInterval = 30 // Explicit timeout
         request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization")
         request.setValue("application/json", forHTTPHeaderField: "Accept")

Also applies to: 1378-1384

---

1219-1224: ClaimsService.getPermission return type inconsistent with Auth.getPermission.

ClaimsService.getPermission(name:) returns Bool, while Auth.getPermission(name:) returns Permission?. This inconsistency may confuse SDK users.

Consider aligning the return types:

     /// Check if a specific permission is granted
     /// - Parameter name: The permission name to check
-    /// - Returns: True if permission is granted, false otherwise
-    public func getPermission(name: String) -> Bool {
-        return auth.getPermission(name: name) != nil
+    /// - Returns: Permission if found, nil otherwise
+    public func getPermission(name: String) -> Permission? {
+        return auth.getPermission(name: name)
     }

Or rename to hasPermission if boolean return is intentional.

---

174-196: Async overloads throw generic NSError instead of typed AuthError.

The async methods with ApiOptions throw NSError with domain "KindeSDK" for API failures, while other parts of the codebase use typed AuthError cases. This inconsistency makes error handling less ergonomic.

Consider adding new AuthError cases and using them consistently:

public enum AuthError: Error {
    // existing cases...
    case apiError(String)
    case permissionsNotFound
    case rolesNotFound
}

Then:

-            throw NSError(domain: "KindeSDK", code: -1, userInfo: [NSLocalizedDescriptionKey: "Failed to fetch permissions from API - check network and authentication"])
+            throw AuthError.apiError("Failed to fetch permissions from API")

Also applies to: 214-231, 250-272, 290-307

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Tests/EntitlementsSpec.swift (2)

269-277: getClaimValue(forKey:) tests may not compile or will always fail without fixtures.

Two separate issues to double‑check here:

1. API existence: From the surrounding context, ClaimsService clearly exposes getClaim(forKey:), but it’s not obvious that a getClaimValue(forKey:) helper actually exists. If it doesn’t, these tests won’t compile. Either:

   • add getClaimValue(forKey:) to ClaimsService (likely returning Any? from the underlying Claim), or
   • rewrite the tests to use getClaim(forKey:) and assert on the Claim’s value.

2. Fixture setup: The "returns claim value when claim exists" example uses "test_claim" without seeding any token/claim, so the result will be nil in the current setup and expect(result).to(beAKindOf(Any.self)) will fail. As with the entitlements tests you already marked pending, you’ll need either a mocked token with that claim or to mark this example pending until fixtures are added.

#!/bin/bash
# Verify whether getClaimValue is actually defined anywhere.
rg -n "getClaimValue" Sources Tests -C2

---

321-355: Async await expect { try await … } blocks in QuickSpec are likely to fail to compile.

These it blocks call await expect { try await … } but EntitlementsSpec currently subclasses QuickSpec and spec() itself is not async. Unless you’re on a Quick version where QuickSpec supports async it closures, this will trigger the usual 'async' call in a function that does not support concurrency build error.

Options:

• Switch this spec to an async variant (e.g., AsyncSpec) and make spec() async so you can safely await inside it closures, or
• Keep QuickSpec but wrap the async calls using a pattern like waitUntil / toEventually, or test your Auth/entitlements HTTP error cases via synchronous stubs instead of real async functions.

Quick & Nimble async: For a Swift test using Quick, is `await expect { try await foo() }` supported inside a `QuickSpec` `it` closure, or must the spec subclass `AsyncSpec` / use an async `spec()` instead?

Tests/EntitlementsSpec.swift (2)

41-42: Align TODO-style comments with SwiftLint and your pending tests.

SwiftLint is warning on these // TODO: Add proper test fixtures… comments. Since the corresponding examples are already marked pending, you could either:

• remove or rephrase the TODO (e.g., reference a ticket ID instead), or
• adjust your SwiftLint config if you intentionally track work via TODOs in tests.

This keeps the lint signal focused on actionable issues.

Also applies to: 264-265

---

57-61: Several tests assert only on static types/fallbacks without seeding entitlements/flags.

In these examples (string-bool/string-numeric entitlements, malformed/null/type-conversion edge cases, hard-check permission/role/flag/entitlement, and “feature flags dictionary when claim exists”), no token/claims/entitlements are set up. The assertions effectively only confirm:

• the functions return their declared types, or
• the documented default value paths work when nothing is present.

They do not currently exercise the “string value”, “malformed claim”, or “claim exists” branches suggested by the test names and comments.

Consider either:

• seeding a mock token / entitlements / feature flags for the specific keys under test, or
• marking these as pending (or renaming them) until fixtures exist, to avoid a misleading sense of coverage.

Also applies to: 76-80, 95-99, 121-124, 126-129, 131-138, 160-165, 172-180, 214-217

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

KindeSDK.xcodeproj/project.pbxproj (1)

126-127: Consider adding Claims and MobileEntitlements to a visible group

Claims.swift and MobileEntitlements.swift have PBXFileReference entries and are compiled, but they don’t appear under any PBXGroup’s children. Adding them (likely to the Auth group) would make them visible in the Xcode navigator and easier to maintain.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro